[flang] Implement `sinpi` #149525

c8ef · 2025-07-18T14:55:10Z

No description provided.

c8ef · 2025-07-18T14:55:31Z

[flang] Implement tanpi #149527
[flang] Implement sinpi #149525 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-07-18T14:57:28Z

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-fir-hlfir

Author: Connector Switch (c8ef)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/149525.diff

4 Files Affected:

(modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+1)
(modified) flang/lib/Evaluate/intrinsics.cpp (+1)
(modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+16)
(added) flang/test/Lower/Intrinsics/sinpi.f90 (+22)

diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index acdba7c49e6b3..d84d3593ebca6 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -419,6 +419,7 @@ struct IntrinsicLibrary {
   mlir::Value genShiftA(mlir::Type resultType, llvm::ArrayRef<mlir::Value>);
   mlir::Value genSign(mlir::Type, llvm::ArrayRef<mlir::Value>);
   mlir::Value genSind(mlir::Type, llvm::ArrayRef<mlir::Value>);
+  mlir::Value genSinpi(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genSize(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genSizeOf(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genSpacing(mlir::Type resultType,
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 9957010684d48..d44239b41fa20 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -957,6 +957,7 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
     {"sin", {{"x", SameFloating}}, SameFloating},
     {"sind", {{"x", SameFloating}}, SameFloating},
     {"sinh", {{"x", SameFloating}}, SameFloating},
+    {"sinpi", {{"x", SameFloating}}, SameFloating},
     {"size",
         {{"array", AnyData, Rank::arrayOrAssumedRank},
             OptionalDIM, // unless array is assumed-size
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index d77a656158a37..823b1eb887992 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -901,6 +901,7 @@ static constexpr IntrinsicHandler handlers[]{
      {{{"number", asValue}, {"handler", asAddr}, {"status", asAddr}}},
      /*isElemental=*/false},
     {"sind", &I::genSind},
+    {"sinpi", &I::genSinpi},
     {"size",
      &I::genSize,
      {{{"array", asBox},
@@ -8060,6 +8061,21 @@ mlir::Value IntrinsicLibrary::genSind(mlir::Type resultType,
   return getRuntimeCallGenerator("sin", ftype)(builder, loc, {arg});
 }
 
+// SINPI
+mlir::Value IntrinsicLibrary::genSinpi(mlir::Type resultType,
+                                       llvm::ArrayRef<mlir::Value> args) {
+  assert(args.size() == 1);
+  mlir::MLIRContext *context = builder.getContext();
+  mlir::FunctionType ftype =
+      mlir::FunctionType::get(context, {resultType}, {args[0].getType()});
+  llvm::APFloat pi = llvm::APFloat(llvm::numbers::pi);
+  mlir::Value dfactor =
+      builder.createRealConstant(loc, mlir::Float64Type::get(context), pi);
+  mlir::Value factor = builder.createConvert(loc, args[0].getType(), dfactor);
+  mlir::Value arg = builder.create<mlir::arith::MulFOp>(loc, args[0], factor);
+  return getRuntimeCallGenerator("sin", ftype)(builder, loc, {arg});
+}
+
 // SIZE
 fir::ExtendedValue
 IntrinsicLibrary::genSize(mlir::Type resultType,
diff --git a/flang/test/Lower/Intrinsics/sinpi.f90 b/flang/test/Lower/Intrinsics/sinpi.f90
new file mode 100644
index 0000000000000..38c2277892ec7
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/sinpi.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-fir %s -o - | FileCheck %s --check-prefixes="CHECK"
+
+function test_real4(x)
+  real :: x, test_real4
+  test_real4 = sinpi(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real4
+! CHECK: %[[dfactor:.*]] = arith.constant 3.1415926535897931 : f64
+! CHECK: %[[factor:.*]] = fir.convert %[[dfactor]] : (f64) -> f32
+! CHECK: %[[mul:.*]] = arith.mulf %{{.*}}, %[[factor]] fastmath<contract> : f32
+! CHECK: %[[sin:.*]] = math.sin %[[mul]] fastmath<contract> : f32
+
+function test_real8(x)
+  real(8) :: x, test_real8
+  test_real8 = sinpi(x)
+end function
+
+! CHECK-LABEL: @_QPtest_real8
+! CHECK: %[[dfactor:.*]] = arith.constant 3.1415926535897931 : f64
+! CHECK: %[[mul:.*]] = arith.mulf %{{.*}}, %[[dfactor]] fastmath<contract> : f64
+! CHECK: %[[sin:.*]] = math.sin %[[mul]] fastmath<contract> : f64

tblah · 2025-07-21T14:40:18Z

flang/lib/Optimizer/Builder/IntrinsicCall.cpp

+  mlir::Value dfactor =
+      builder.createRealConstant(loc, mlir::Float64Type::get(context), pi);
+  mlir::Value factor = builder.createConvert(loc, args[0].getType(), dfactor);


Why not create the real constant with the argument type in the first place? This might matter for real(16).

mlir::Value factor = builder.createRealConstant(loc, args[0].getType(), pi);

Using the argument type in the creation process seems to result in the following error:

error: FloatAttr type doesn't match the type implied by its value flang: /home/c8ef/llvm-dev/mlir/include/mlir/IR/StorageUniquerSupport.h:180: static ConcreteT mlir::detail::StorageUserBase<mlir::FloatAttr, mlir::Attribute, mlir::detail::FloatAttrStorage, mlir::detail::AttributeUniquer, mlir::TypedAttr::Trait>::get(MLIRContext *, Args &&...) [ConcreteT = mlir::FloatAttr, BaseT = mlir::Attribute, StorageT = mlir::detail::FloatAttrStorage, UniquerT = mlir::detail::AttributeUniquer, Traits = <mlir::TypedAttr::Trait>, Args = <mlir::Type &, const llvm::APFloat &>]: Assertion `succeeded( ConcreteT::verifyInvariants(getDefaultDiagnosticEmitFn(ctx), args...))' failed. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. Program arguments: /home/c8ef/llvm-dev/build/bin/flang -fc1 -emit-fir /home/c8ef/llvm-dev/flang/test/Lower/Intrinsics/sinpi.f90 -o - #0 0x0000ffffa6de9e50 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:834:11 #1 0x0000ffffa6dea368 PrintStackTraceSignalHandler(void*) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:918:1 #2 0x0000ffffa6de83f4 llvm::sys::RunSignalHandlers() /home/c8ef/llvm-dev/llvm/lib/Support/Signals.cpp:104:5 #3 0x0000ffffa6deab18 SignalHandler(int, siginfo_t*, void*) /home/c8ef/llvm-dev/llvm/lib/Support/Unix/Signals.inc:426:38 #4 0x0000ffffaa031808 (linux-vdso.so.1+0x808) #5 0x0000ffffa64d7dc0 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 #6 0x0000ffffa6486980 raise ./signal/../sysdeps/posix/raise.c:27:6 #7 0x0000ffffa6471ac4 abort ./stdlib/abort.c:81:3 #8 0x0000ffffa647f9bc __assert_fail_base ./assert/assert.c:53:15 #9 0x0000ffff9fc572cc mlir::FloatAttr mlir::detail::StorageUserBase<mlir::FloatAttr, mlir::Attribute, mlir::detail::FloatAttrStorage, mlir::detail::AttributeUniquer, mlir::TypedAttr::Trait>::get<mlir::Type&, llvm::APFloat const&>(mlir::MLIRContext*, mlir::Type&, llvm::APFloat const&) /home/c8ef/llvm-dev/mlir/include/mlir/IR/StorageUniquerSupport.h:179:5 #10 0x0000ffff9fc4c8c8 mlir::FloatAttr::get(mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/build/tools/mlir/include/mlir/IR/BuiltinAttributes.cpp.inc:293:10 #11 0x0000ffff9fc3b3bc mlir::Builder::getFloatAttr(mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/mlir/lib/IR/Builders.cpp:254:10 #12 0x0000ffff944e0b84 fir::FirOpBuilder::createRealConstant(mlir::Location, mlir::Type, llvm::APFloat const&) /home/c8ef/llvm-dev/flang/lib/Optimizer/Builder/FIRBuilder.cpp:187:17 #13 0x0000ffff94567eac fir::IntrinsicLibrary::genSinpi(mlir::Type, llvm::ArrayRef<mlir::Value>) /home/c8ef/llvm-dev/flang/lib/Optimizer/Builder/IntrinsicCall.cpp:8072:32

LogicalResult FloatAttr::verify(function_ref<InFlightDiagnostic()> emitError, Type type, APFloat value) { // Verify that the type is correct. if (!llvm::isa<FloatType>(type)) return emitError() << "expected floating point type"; // Verify that the type semantics match that of the value. if (&llvm::cast<FloatType>(type).getFloatSemantics() != &value.getSemantics()) { return emitError() << "FloatAttr type doesn't match the type implied by its value"; } return success(); }

It appears these two floating-point types have different semantics. To ensure pi uses the same semantics as the argument type, we'd still need to convert it from APFloat. Either way, a conversion seems necessary.

Ahh okay what I think is happening here is that APFloat is forcing its size to f64 because llvm::numbers::pi is defined as a double. I'm concerned that this could produce inaccurate results for real(16) (aka floating point types with more precision than f64: f128 on aarch64 systems, I think f80 on x86).

Yeah so I think the ultimate solution for trig-pi functions is to use the glibc version, when we have glibc>=2.41(it does provide f128 version). And if not maybe we can use this as a fallback or something like libquadmath. Or maybe we need a interface return pi based on fltSemantic. Not quite sure how to do it right since there are a lot of them.

Yes, handling these is somewhat tricky. I'm not exactly sure how to handle this correctly in flang. I am currently working on adding sinpi-related functions to libquadmath in the GCC source tree. The GCC maintainers are not satisfied with the precision of the generic implementation imported from glibc, so this patch has not been merged yet.

C23 has added support for sinpi-related functions, and eventually libc will also have support. This will allow us to achieve better precision. If neither glibc nor libquadmath have it, we can fallback to our current approach.

However, glibc does not provide sind-related functions. We just need to fix the constant pi type later.

I think you may follow the implementation of SinF128 as Tom suggested. As long as libquadmath does not support sinpi for float128 you may implement SinpiF128 in flang-rt/lib/quadmath same way you implement it here but using proper PI definition from https://gcc.gnu.org/onlinedocs/libquadmath/Typedef-and-constants.html

I think this is a good approach for handling the f128 case. However, if we implement this logic in libquadmath, will it diverge from how we handle f32/f64 here?

Diverge in what sense?

If we "lower" the f32/f64 cases here as sin(PI * x), and the f128 case in Flang's quadmath as sinq(M_PIq * x), then they should be pretty consistent, right?

Ah, you're right. I get it. That seems like a good approach.

tblah

Thanks for the patch!

c8ef · 2025-07-23T00:21:36Z

Merge activity

Jul 23, 12:21 AM UTC: A user started a stack merge that includes this pull request via Graphite.
Jul 23, 12:23 AM UTC: @c8ef merged this pull request with Graphite.

[flang] Implement sinpi

1e9f5c1

c8ef changed the title ~~[flang] Implement sinpi~~ [flang] Implement sinpi Jul 18, 2025

c8ef marked this pull request as ready for review July 18, 2025 14:56

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Jul 18, 2025

c8ef mentioned this pull request Jul 18, 2025

[flang] Implement tanpi #149527

Merged

c8ef requested review from clementval and tblah July 18, 2025 15:06

tblah reviewed Jul 21, 2025

View reviewed changes

c8ef requested a review from tblah July 21, 2025 15:51

tblah approved these changes Jul 22, 2025

View reviewed changes

c8ef merged commit bbbe69f into main Jul 23, 2025
15 checks passed

c8ef deleted the users/c8ef/_flang_implement_sinpi branch July 23, 2025 00:23

c8ef mentioned this pull request Jul 24, 2025

[flang] Create the real constant directly with the argument type. #150452

Open

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025

[flang] Implement sinpi (llvm#149525)

fee8f2d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang] Implement `sinpi` #149525

[flang] Implement `sinpi` #149525

Uh oh!

c8ef commented Jul 18, 2025

Uh oh!

c8ef commented Jul 18, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 18, 2025 •

edited

Loading

Uh oh!

tblah Jul 21, 2025

Uh oh!

c8ef Jul 21, 2025

Uh oh!

c8ef Jul 21, 2025

Uh oh!

tblah Jul 21, 2025

Uh oh!

c8ef Jul 21, 2025

Uh oh!

c8ef Jul 22, 2025

Uh oh!

vzakhari Jul 22, 2025

Uh oh!

c8ef Jul 22, 2025

Uh oh!

vzakhari Jul 22, 2025

Uh oh!

c8ef Jul 23, 2025

Uh oh!

tblah left a comment

Uh oh!

c8ef commented Jul 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

[flang] Implement sinpi #149525

[flang] Implement sinpi #149525

Uh oh!

Conversation

c8ef commented Jul 18, 2025

Uh oh!

c8ef commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

c8ef commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Uh oh!

[flang] Implement `sinpi` #149525

[flang] Implement `sinpi` #149525

c8ef commented Jul 18, 2025 •

edited

Loading

llvmbot commented Jul 18, 2025 •

edited

Loading

c8ef commented Jul 23, 2025 •

edited

Loading